-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
create ssl cert per vhost, not one monster #2129
Conversation
2f6f03b
to
e86d51f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tl;dr: 👍 on the concept, some implementation notes inline.
I recently read there's some benefit in having all names on a certificate so browsers can reuse connections, but given we're mostly behind a CDN I think the benefit of that is really limited and the operational simplicity of one cert per vhost is way more important.
In the installer we use datacat to gather all values so you could do it that way per host, but it's complex. Also, the module is unmaintained and collections is a maintained alternative.
This solution works for me as an approach. Thanks for taking a look. |
finally updated! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but @ekohl should give the final stamp of approval
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we could migrate this to using web::vhost
as well after this change.
https://webmasters.stackexchange.com/questions/97005/setting-x-forwarded-proto-under-apache-2-4 suggests you can also use RequestHeader set X-Forwarded-Proto expr=%{REQUEST_SCHEME}
to make it dynamic, meaning it's going to be the same directive for HTTP and HTTPS.
I can submit a follow up PR to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.